-
-
Notifications
You must be signed in to change notification settings - Fork 51
[Platform] Add support for Google vertex AI #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Platform] Add support for Google vertex AI #297
Conversation
14ae923
to
35d9a8b
Compare
src/platform/composer.json
Outdated
@@ -22,6 +22,7 @@ | |||
"require": { | |||
"php": ">=8.2", | |||
"ext-fileinfo": "*", | |||
"google/auth": "^1.47", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it to require-dev
and add a check for the class existence to the implementation, like in the Bedrock one for example:
ai/src/platform/src/Bridge/Bedrock/PlatformFactory.php
Lines 37 to 39 in 11ef98f
if (!class_exists(BedrockRuntimeClient::class)) { | |
throw new RuntimeException('For using the Bedrock platform, the async-aws/bedrock-runtime package is required. Try running "composer require async-aws/bedrock-runtime".'); | |
} |
35d9a8b
to
9124195
Compare
?Contract $contract = null, | ||
): Platform { | ||
if (!class_exists(ApplicationDefaultCredentials::class)) { | ||
throw new RuntimeException('For using the Bedrock platform, the google/auth package is required. Try running "composer require google/auth".'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new RuntimeException('For using the Bedrock platform, the google/auth package is required. Try running "composer require google/auth".'); | |
throw new RuntimeException('For using the Vertex AI platform, google/auth package is required for authentication via application default credentials. Try running "composer require google/auth".'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it already.
Pushing the change in a few moments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, ship it
9124195
to
cd83b3b
Compare
Docs and AI bundle integration need to be added first. |
0cf0db1
to
1346440
Compare
@chr-hertel |
- Adds initial support to integrate text generation using vertex AI
- Adds support for embeddings
- Adds tests to verify the behavior
- Adds examples to interact with vertex ai
- Adds ai bundle integration
- Adds documentation for vertex ai
1346440
to
b08d7e7
Compare
* } | ||
* } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indention looks wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PHP doc, the IDE doesn't apparently help with the indentation in this case.
I tried to correct the indentation now.
* file that was distributed with this source code. | ||
*/ | ||
|
||
use Google\Auth\ApplicationDefaultCredentials; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs a new require in the examples/composer.json
file - cannot run the examples locally at this point, sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a biggie.
I did think about it, but somehow it skipped my mind, probably because the examples worked for me as I included the autoload file in another way.
src/ai-bundle/doc/index.rst
Outdated
@@ -52,6 +52,9 @@ Configuration | |||
api_version: '%env(AZURE_GPT_VERSION)%' | |||
gemini: | |||
api_key: '%env(GEMINI_API_KEY)%' | |||
vertexAi: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be
vertexAi: | |
vertex_ai: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it require a change inside AiBundle.php
too? While integrating the bridge into the AI bundle, I looked for other bridges that had multiple words in them and found that they were added as a single word. And I followed suit.
I did, however, change it to vertexai.
Given your input on it, however, I'd be happy to change it to vertex_ai.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a similar doc to the gemini one, please combine the two files a la Server Tools - supported by VertexAI and Gemini API or sth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this on purpose so that when we later make a decision about whether we need both the bridges or only Vertex AI, then we could make such a refactor.
src/ai-bundle/doc/index.rst
Outdated
@@ -34,7 +34,7 @@ Configuration | |||
class: 'Symfony\AI\Platform\Bridge\OpenAi\Gpt' | |||
name: !php/const Symfony\AI\Platform\Bridge\OpenAi\Gpt::GPT_4O_MINI | |||
|
|||
**Advanced Example with Anthropic, Azure, Gemini and multiple agents** | |||
**Advanced Example with Anthropic, Azure, Gemini, VertexAi and multiple agents** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the official product name is "Vertex AI" i guess
**Advanced Example with Anthropic, Azure, Gemini, VertexAi and multiple agents** | |
**Advanced Example with Anthropic, Azure, Gemini, Vertex AI and multiple agents** |
src/platform/doc/index.rst
Outdated
@@ -73,13 +73,15 @@ usually defined by the specific models and their documentation. | |||
* `OpenAI's GPT`_ with `OpenAI`_ and `Azure`_ as Platform | |||
* `Anthropic's Claude`_ with `Anthropic`_ and `AWS Bedrock`_ as Platform | |||
* `Meta's Llama`_ with `Azure`_, `Ollama`_, `Replicate`_ and `AWS Bedrock`_ as Platform | |||
* `Gemini`_ with `Google`_ and `OpenRouter`_ as Platform | |||
* `Gemini`_ with `Google`_, `VertexAi`_ and `OpenRouter`_ as Platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `Gemini`_ with `Google`_, `VertexAi`_ and `OpenRouter`_ as Platform | |
* `Gemini`_ with `Google`_, `Vertex AI`_ and `OpenRouter`_ as Platform |
Thanks @junaidbinfarooq for your work on this - I think we're getting somewhere. In general it great to have that bridge, I still wonder tho about the amount of duplicated code with the Gemini bridge and we would need to support API Keys as well ... but both topics are fine as follow up |
@chr-hertel |
- Adds support to calculate the token usage
b08d7e7
to
605bf5e
Compare
Changes proposed: